Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert/package faster! #68

Merged
merged 4 commits into from
Feb 28, 2024
Merged

Convert/package faster! #68

merged 4 commits into from
Feb 28, 2024

Conversation

nirvn
Copy link
Member

@nirvn nirvn commented Feb 24, 2024

This pull request dramatically (maybe I'm being a bit too... dramatic 😉 ) increase packaging speed in large projects (or projects containing slow remote data sources) by avoiding the cost of a full project load when merely wanting to fetch the map canvas XML.

In addition, a new convert() function parameter allows skipping of a final project reload when not needed (i.e., when packaging projects on the cloud).

@nirvn nirvn force-pushed the convert_faster branch 3 times, most recently from 9081641 to 638a6f6 Compare February 24, 2024 04:15
Copy link
Collaborator

@suricactus suricactus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Big +1 for the QgsProject.read() flags approach. However, I am not convinced on the temporary QgsProject and file approach. Can you elaborate what you are trying to achieve by introducing them?

@nirvn
Copy link
Member Author

nirvn commented Feb 24, 2024

@suricactus , as mentioned in my reply to your comment above, the introduction of a temporary QgsProject is done so we can use the QgsProject.FlagDontResolveLayers , which makes project loading blazingly fast. Since we're only looking for the map canvas XML element, we can make use of that flag and say bye bye to long project load.

If we would be doing this on the main QgsProject.instance() singleton, we would be unable to then use the project to manipulate layers.

@suricactus
Copy link
Collaborator

Thanks for the explanations. Can we just put them in the code as I attempted in #69 . I tried putting my understanding there.

@nirvn
Copy link
Member Author

nirvn commented Feb 24, 2024

@suricactus , good idea. You want to push your commit into this PR or we merge both sequentially?

@suricactus
Copy link
Collaborator

If you agree with my proposed changes in #69, when you merge, they will come into this PR. On my PR I set the base branch to be your branch here.

@nirvn nirvn merged commit fefa28f into master Feb 28, 2024
7 checks passed
@nirvn nirvn deleted the convert_faster branch February 28, 2024 01:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants